Skip to content

Fix cgroup helper init in add_processor_metadata and add_docker_metadata processors#41108

Merged
fearful-symmetry merged 5 commits intoelastic:mainfrom
fearful-symmetry:processor-cgroup-perf-fixes
Oct 9, 2024
Merged

Fix cgroup helper init in add_processor_metadata and add_docker_metadata processors#41108
fearful-symmetry merged 5 commits intoelastic:mainfrom
fearful-symmetry:processor-cgroup-perf-fixes

Conversation

@fearful-symmetry
Copy link
Contributor

@fearful-symmetry fearful-symmetry commented Oct 3, 2024

Proposed commit message

Closes #41065

This refactors add_processor_metadata and add_docker_metadata so we only call NewReader once when we create a new processor, and not every time the processor runs. This should considerably cut down on CPU cycles for certain workloads, since NewReader has gotten much more complex over the years.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@fearful-symmetry fearful-symmetry added the Team:Security-Linux Platform Linux Platform Team in Security Solution label Oct 3, 2024
@fearful-symmetry fearful-symmetry self-assigned this Oct 3, 2024
@fearful-symmetry fearful-symmetry requested a review from a team as a code owner October 3, 2024 21:47
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 3, 2024

Pinging @elastic/sec-linux-platform (Team:Security-Linux Platform)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 3, 2024
@mergify
Copy link
Contributor

mergify bot commented Oct 3, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @fearful-symmetry? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit

@mergify
Copy link
Contributor

mergify bot commented Oct 3, 2024

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Oct 3, 2024
@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Oct 4, 2024
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 4, 2024

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

belimawr
belimawr approved these changes Oct 4, 2024
View reviewed changes
Copy link
Contributor

@belimawr belimawr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not my area of expertise, but LGTM.

I've seen the same CI error transaction_log_integration_test.go:22: timeout waiting for services to be healthy in one of my PRs, so I believe it's related to this one.

@belimawr belimawr mentioned this pull request Oct 4, 2024
Closed
@fearful-symmetry fearful-symmetry merged commit 13d71a6 into elastic:main Oct 9, 2024
mergify bot pushed a commit that referenced this pull request Oct 9, 2024
@fearful-symmetry @pierrehilbert @mergify
Fix cgroup helper init in add_processor_metadata and `add_docker_me…
fedef4d 
…tadata` processors (#41108)

* only initialize cgroup reader once

* docs

* remove old struct fields

---------

Co-authored-by: Pierre HILBERT <pierre.hilbert@elastic.co>
(cherry picked from commit 13d71a6)
@mergify mergify bot mentioned this pull request Oct 9, 2024
Merged
4 tasks
@rockdaboot
Copy link
Contributor

rockdaboot commented Oct 9, 2024

@fearful-symmetry Thank you so much!

@Kavindu-Dodan
Copy link
Contributor

Kavindu-Dodan commented Oct 9, 2024 •
edited
Loading

This PR seems to break MAC os development of beats if any of the metadata processors are enabled. In my setup, I get Exiting: error initializing processors: error creating cgroup reader: cgroups not found or unsupported by OS [1]

[1] - https://github.com/elastic/beats/pull/41108/files#diff-5fb40da34716c19b3759ec0b54acbafe4f88a27fdf822593b56683343d35089eR166-R169

@fearful-symmetry fearful-symmetry mentioned this pull request Oct 9, 2024
Merged
4 tasks
@mergify mergify bot mentioned this pull request Oct 10, 2024
Closed
4 tasks
pierrehilbert pushed a commit that referenced this pull request Oct 11, 2024
@mergify @fearful-symmetry
Fix cgroup helper init in add_processor_metadata and `add_docker_me…
e6e4f5f 
…tadata` processors (#41108) (#41185)

* only initialize cgroup reader once

* docs

* remove old struct fields

---------

Co-authored-by: Pierre HILBERT <pierre.hilbert@elastic.co>
(cherry picked from commit 13d71a6)

Co-authored-by: Alex K. <8418476+fearful-symmetry@users.noreply.github.com>
@khushijain21 khushijain21 mentioned this pull request Jun 23, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@belimawr belimawr belimawr approved these changes

@faec faec Awaiting requested review from faec faec is a code owner automatically assigned from elastic/elastic-agent-data-plane

Assignees

@fearful-symmetry fearful-symmetry

Labels

backport-8.x Automated backport to the 8.x branch with mergify Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team Team:Security-Linux Platform Linux Platform Team in Security Solution

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add_process_metadata and and_docker_metadata shouldn't initialize a cgroup reader more than once

6 participants

@fearful-symmetry @elasticmachine @rockdaboot @Kavindu-Dodan @belimawr @pierrehilbert